Skip to content

Conversation

@jem-davies
Copy link
Collaborator

@jem-davies jem-davies commented Jan 8, 2026

Some context: I had decided to implement this as a simpler alternative to #514.

Signed-off-by: Jem Davies <jemsot@gmail.com>
Signed-off-by: Jem Davies <jemsot@gmail.com>
Signed-off-by: Jem Davies <jemsot@gmail.com>
Signed-off-by: Jem Davies <jemsot@gmail.com>
Signed-off-by: Jem Davies <jemsot@gmail.com>
Replace errChan draining with doneChan synchronization to ensure
watch() goroutine exits before closing file handle.

Also fix file handle leaks in newTail() error paths.
Signed-off-by: Jem Davies <jemsot@gmail.com>

if !os.SameFile(t.fileInfo, tempInfo) {
if t.logger != nil {
t.logger.Info(fmt.Sprintf("Handling rotation for %v", t.path))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we downgrade these rotation logs to the debug level?


pos, err := t.file.Seek(0, io.SeekCurrent)
if err != nil {
return fmt.Errorf("seek file: %w", err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps some of these errors should have more info added:

Suggested change
return fmt.Errorf("seek file: %w", err)
return fmt.Errorf("failed to set seek on %s: %w", t.file.Name(). , err)

Comment on lines +291 to +292
line, err := t.reader.ReadBytes('\n')
line = bytes.TrimRight(line, "\r\n")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason why we're exclusively using \n as a delimiter here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes could have a config option for that

Comment on lines +188 to +189
file *os.File
fileInfo os.FileInfo
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on using the *service.FS from the mgr *service.Resources instead for file reading?

i.e

mgr.FS().OpenFile()

This way we're relying on our FS fixtures instead of the os package

}
}

func (fti *FileTailInput) Close(ctx context.Context) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this idempotent? Also, we should probably select on the ctx.Done() to allow returning early if the ctx is cancelled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants